-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implementing targets for SpatRasterCollection: tar_terra_sprc()
#50
Conversation
* For use with multiple raster files, which are saved as a `SpatRasterCollection` object * Tests (surprisingly?) Pass * Need to address some issues of how GDAL options are used/passed/manipulated * 🚀 🚀 🚀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with merging this as-is, especially if it is helping you get work done. However, what I really wish for is a way to use sprc()
and terra::c()
with dynamic and static branching. E.g., would the test pipeline in this PR perhaps be better done with tar_map()
and a geotargets
version of tarchetypes::tar_combine()
like tar_combine_sprc()
? Or would it be possible to create our own options for an iterate
argument so one could use dynamic branching with iterate = "sprc"
or iterate = "SpatRaster_layers"
? I think we might have to dig more into the targets
code or ask Will to answer those questions.
R/tar-terra-rasts.R
Outdated
.write_terra_rasters_sprc <- eval( | ||
substitute( | ||
function(object, path) { | ||
for (i in seq(object)) { | ||
if (i > 1) { | ||
opt <- "APPEND_SUBDATASET=YES" | ||
} else { | ||
opt <- "" | ||
} | ||
terra::writeRaster( | ||
x = object[i], | ||
filename = path, | ||
filetype = filetype, | ||
overwrite = (i == 1), | ||
gdal = opt | ||
) | ||
} | ||
}, | ||
list(filetype = filetype, gdal = gdal) | ||
) | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it might be more useful to pull this out and create a write_sprc()
function? Would it work on any SpatRasterCollection outside of a targets pipeline?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm interesting point! I'm not sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if this is something we should add, in terms of reading/writing function outside of geotargets, we should let the targets manage that work.
Re
I think that we should spin these issues out in to a separate issue. How all this works with static and dynamic branching is really important to consider, but I think we should approach those features separately. |
…d we should make that clear
tar_terra_rasts()
tar_terra_sprc()
SpatRasterCollection
object